Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

poly: edit multiply and add divide functions #213

Closed
wants to merge 22 commits into from

Conversation

suleyman-kaya
Copy link
Contributor

@suleyman-kaya suleyman-kaya commented Aug 3, 2024

poly: add multiply and divide functions

Description:

This pull request introduces two new functions, multiply and divide, to the poly module for polynomial arithmetic. The multiply function allows for the multiplication of two polynomials, while the divide function performs polynomial long division, returning both the quotient and remainder.

Changes:

  • Added multiply function:

    • Multiplies two polynomials represented as arrays of coefficients.
    • Input: two arrays [a_0, ..., a_n] and [b_0, ..., b_m].
    • Output: a new array [c_0, ..., c_{n+m}] where each coefficient c_k is computed as the sum of products of corresponding coefficients from the input polynomials.
  • Added divide function:

    • Divides one polynomial by another using the polynomial long division algorithm.
    • Input: two arrays [a_0, ..., a_n] and [b_0, ..., b_m].
    • Output: a tuple (q, r) where q is the quotient and r is the remainder such that a(x) = b(x) * q(x) + r(x) and the degree of r is less than the degree of b.
  • Updated test files to include tests for the multiply and divide functions.

Functions:

/* Multiplies two polynomials: (a_n * x^n + ... + a_0) * (b_m * x^m + ... + b_0)
   Input: a = [a_0, ..., a_n], b = [b_0, ..., b_m]
   Output: [c_0, c_1, ..., c_{n+m}] where c_k = ∑_{i+j=k} a_i * b_j
*/
pub fn multiply(a []f64, b []f64) []f64 {
	mut result := []f64{len: a.len + b.len - 1}
	for i in 0 .. a.len {
		for j in 0 .. b.len {
			result[i + j] += a[i] * b[j]
		}
	}
	return result
}
/* Divides two polynomials: (a_n * x^n + ... + a_0) / (b_m * x^m + ... + b_0)
   Uses polynomial long division algorithm
   Input: a = [a_0, ..., a_n], b = [b_0, ..., b_m]
   Output: (q, r) where q is the quotient and r is the remainder
   such that a(x) = b(x) * q(x) + r(x) and degree(r) < degree(b)
*/
pub fn divide(a []f64, b []f64) ([]f64, []f64) {
	mut quotient := []f64{}
	mut remainder := a.clone()
	b_lead_coef := b[0]

	for remainder.len >= b.len {
		lead_coef := remainder[0] / b_lead_coef
		quotient << lead_coef
		for i in 0 .. b.len {
			remainder[i] -= lead_coef * b[i]
		}
		remainder = remainder[1..]
		for remainder.len > 0 && math.abs(remainder[0]) < 1e-10 {
			remainder = remainder[1..]
		}
	}

	if remainder.len == 0 {
		remainder = []f64{}
	}

	return quotient, remainder
}

Tests:

fn test_multiply() {
	// (2+3x+4x^2) * (-3x+2x^2) = (-6x -5x^2 -6x^3 + 8x^4)
	a := [2.0, 3.0, 4.0]
	b := [0.0, -3.0, 2.0]
	result := multiply(a, b)
	println('Multiply result: ${result}')
	assert result == [0.0, -6.0, -5.0, -6.0, 8.0]
}
fn test_divide() {
	// (x^2 + 2x + 1) / (x + 1) = (x + 1)
	a := [1.0, 2.0, 1.0]
	b := [1.0, 1.0]
	quotient, remainder := divide(a, b)
	println('Divide quotient: ${quotient}')
	println('Divide remainder: ${remainder}')
	assert quotient == [1.0, 1.0]
	assert remainder == []
}

Summary by CodeRabbit

  • New Features

    • Added new functions for polynomial operations: add, subtract, multiply, and divide.
    • Introduced companion_matrix and balance_companion_matrix functions for enhanced polynomial functionality.
    • Implemented a comprehensive Decision Tree algorithm for classification tasks.
    • Added a Random Forest module providing an ensemble learning method with multiple decision trees.
    • Introduced a Support Vector Machine (SVM) framework for binary and multiclass classification.
  • Improvements

    • Comprehensive documentation has been added to all public functions, improving usability.
    • Updated user documentation for clarity and consistency regarding polynomial functions.
    • Enhanced explanations and structure within the README for polynomial evaluation and root-finding functions.
    • Test functions have been refined for clarity and correctness, with the introduction of new tests for multiplication and division.
  • Bug Fixes

    • Corrected a typographical error in the test function name from test_substract to test_subtract.

Copy link

coderabbitai bot commented Aug 3, 2024

Walkthrough

The recent updates significantly enhance polynomial operations and introduce robust implementations for Decision Trees, Random Forests, and Support Vector Machines (SVMs). New functions improve polynomial calculations and testing, while comprehensive documentation clarifies usage. Additionally, a suite of unit tests ensures functionality across all components, contributing to a more reliable and user-friendly experience for developers working with mathematical models.

Changes

Files Change Summary
poly/poly.v, poly/poly_test.v, poly/README.md Enhanced polynomial operations with new functions (add, subtract, multiply, divide), improved documentation for clarity, and added explanations on companion matrices and testing improvements.
ml/decision_tree.v, ml/decision_tree_test.v Introduced a Decision Tree implementation with training and prediction capabilities, alongside a suite of tests to validate functionality.
ml/random_forest.v, ml/random_forest_test.v Added a Random Forest algorithm with training and prediction features, including a testing framework for validation.
ml/svm.v, ml/svm_test.v Implemented a Support Vector Machine framework with various kernel functions, complemented by tests to ensure accuracy and performance.

Poem

🐇 In fields of math, we hop and play,
New functions bloom, bright as the day.
Addition and more, we dance with glee,
Polynomial magic, come join me!
With tests so clever and doc to guide,
Let’s leap through numbers, side by side! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (4)
poly/poly_test.v (4)

25-25: Remove unnecessary print statement.

The print statement is not needed in a test function and should be removed to keep the test output clean.

-  println('Add result: ${result}')

33-33: Remove unnecessary print statement.

The print statement is not needed in a test function and should be removed to keep the test output clean.

-  println('Subtract result: ${result}')

42-42: Remove unnecessary print statement.

The print statement is not needed in a test function and should be removed to keep the test output clean.

-  println('Multiply result: ${result}')

51-52: Remove unnecessary print statements.

The print statements are not needed in a test function and should be removed to keep the test output clean.

-  println('Divide quotient: ${quotient}')
-  println('Divide remainder: ${remainder}')
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b691246 and 84ec369.

Files selected for processing (2)
  • poly/poly.v (10 hunks)
  • poly/poly_test.v (2 hunks)
Additional comments not posted (14)
poly/poly_test.v (2)

Line range hint 7-13:
LGTM!

The test function is well-written and correctly tests the evaluation of a polynomial.


Line range hint 16-20:
LGTM!

The test function is well-written and correctly tests the swapping of two numbers.

poly/poly.v (12)

Line range hint 13-24:
LGTM!

The function is well-documented and correctly implements polynomial evaluation using Horner's method.


Line range hint 29-59:
LGTM!

The function is well-documented and correctly implements the evaluation of a polynomial and its derivatives.


Line range hint 65-91:
LGTM!

The function is well-documented and correctly implements the solution of a quadratic equation using the quadratic formula.


Line range hint 97-132:
LGTM!

The function is well-documented and correctly implements the solution of a cubic equation using Cardano's formula and trigonometric solution.


137-141: LGTM!

The function is well-documented and correctly implements the swapping of two numbers.


Line range hint 146-162:
LGTM!

The function is well-documented and correctly implements the sorting of three numbers in ascending order.


Line range hint 173-190:
LGTM!

The function is well-documented and correctly implements the creation of a companion matrix for a polynomial.


Line range hint 196-256:
LGTM!

The function is well-documented and correctly implements the balancing of a companion matrix to improve numerical stability.


261-267: LGTM!

The function is well-documented and correctly implements the addition of two polynomials.


273-279: LGTM!

The function is well-documented and correctly implements the subtraction of two polynomials.


285-293: LGTM!

The function is well-documented and correctly implements the multiplication of two polynomials.


301-323: LGTM!

The function is well-documented and correctly implements the division of two polynomials using polynomial long division.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (9)
poly/README.md (9)

29-29: Clarify the output array description.

The description of the output array for eval_derivs can be made clearer by explicitly stating that the array contains the polynomial value and its derivatives.

- The output array contains the values of `d^k P(x)/d x^k` for the specified value of `x`, starting with `k = 0`.
+ The output array contains the polynomial value and its derivatives, specifically the values of `d^k P(x)/d x^k` for the specified value of `x`, starting with `k = 0`.

43-43: Use an en dash for numerical ranges.

Consider using an en dash to indicate numerical ranges for better readability.

- The number of real roots (either zero, one, or two) is returned, and their locations are returned as `[ x0, x1 ]`.
+ The number of real roots (either zero, one, or two) is returned, and their locations are returned as `[ x0, x1 ]`.

59-59: Remove unnecessary hyphen.

The hyphen in "closely-spaced" is not necessary.

- finite precision may cause equal or closely-spaced real roots to move off the real axis into the complex plane
+ finite precision may cause equal or closely spaced real roots to move off the real axis into the complex plane
Tools
LanguageTool

[style] ~59-~59: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...se, finite precision may cause equal or closely-spaced real roots to move off the real axis in...

(HYPHENATED_LY_ADVERB_ADJECTIVE)


75-75: Specify the language for the code block.

The fenced code block should have a language specified for better syntax highlighting.

- ```
+ ```console
Tools
Markdownlint

75-75: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


97-97: Remove trailing spaces.

There is a trailing space that should be removed.

- Adds two polynomials: 
+ Adds two polynomials:
Tools
Markdownlint

97-97: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)


103-103: Clarify the result description for add.

The description of the result for the add function can be made clearer by explicitly stating that the result is a polynomial with the summed coefficients.

- Returns the result as `[a_0 + b_0, a_1 + b_1, ..., max(a_k, b_k), ...]`.
+ Returns the result as a polynomial with the summed coefficients `[a_0 + b_0, a_1 + b_1, ..., max(a_k, b_k), ...]`.

115-115: Clarify the result description for subtract.

The description of the result for the subtract function can be made clearer by explicitly stating that the result is a polynomial with the subtracted coefficients.

- Returns the result as `[a_0 - b_0, a_1 - b_1, ..., a_k - b_k, ...]`.
+ Returns the result as a polynomial with the subtracted coefficients `[a_0 - b_0, a_1 - b_1, ..., a_k - b_k, ...]`.

127-127: Clarify the result description for multiply.

The description of the result for the multiply function can be made clearer by explicitly stating that the result is a polynomial with the multiplied coefficients.

- Returns the result as `[c_0, c_1, ..., c_{n+m}]` where `c_k = ∑_{i+j=k} a_i * b_j`.
+ Returns the result as a polynomial with the multiplied coefficients `[c_0, c_1, ..., c_{n+m}]` where `c_k = ∑_{i+j=k} a_i * b_j`.

139-139: Clarify the result description for divide.

The description of the result for the divide function can be made clearer by explicitly stating that the result is a tuple containing the quotient and remainder polynomials.

- Uses polynomial long division algorithm. Returns `(q, r)` where `q` is the quotient and `r` is the remainder such that `a(x) = b(x) * q(x) + r(x)` and `degree(r) < degree(b)`.
+ Uses polynomial long division algorithm. Returns a tuple `(q, r)` where `q` is the quotient polynomial and `r` is the remainder polynomial such that `a(x) = b(x) * q(x) + r(x)` and `degree(r) < degree(b)`.
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 84ec369 and e578ca5.

Files selected for processing (1)
  • poly/README.md (3 hunks)
Additional context used
LanguageTool
poly/README.md

[typographical] ~45-~45: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...depends on the sign of the discriminant b^2 - 4 a c. This will be subject to rounding...

(DASH_RULE)


[style] ~59-~59: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...se, finite precision may cause equal or closely-spaced real roots to move off the real axis in...

(HYPHENATED_LY_ADVERB_ADJECTIVE)

Markdownlint
poly/README.md

97-97: Expected: 0 or 2; Actual: 1
Trailing spaces

(MD009, no-trailing-spaces)


75-75: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Additional comments not posted (1)
poly/README.md (1)

89-89: LGTM!

The description of the balance_companion_matrix function is clear and accurate.

@suleyman-kaya suleyman-kaya changed the title poly: add multiply and divide functions poly: edit multiply and add divide functions Aug 3, 2024
Copy link
Member

@ulises-jeremias ulises-jeremias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some changes, the rest looks great!

poly/poly.v Outdated
using Horner's method: P(x) = (...((a_n * x + a_{n-1}) * x + a_{n-2}) * x + ... + a_1) * x + a_0
Input: c = [a_0, a_1, ..., a_n], x
Output: P(x)
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you replace all this comments

/* Evaluates a polynomial P(x) = a_n * x^n + a_{n-1} * x^{n-1} + ... + a_1 * x + a_0
   using Horner's method: P(x) = (...((a_n * x + a_{n-1}) * x + a_{n-2}) * x + ... + a_1) * x + a_0
   Input: c = [a_0, a_1, ..., a_n], x
   Output: P(x)
*/

to

// eval is a function that evaluates a polynomial P(x) = a_n * x^n + a_{n-1} * x^{n-1} + ... + a_1 * x + a_0
// using Horner's method: P(x) = (...((a_n * x + a_{n-1}) * x + a_{n-2}) * x + ... + a_1) * x + a_0
// Input: c = [a_0, a_1, ..., a_n], x
// Output: P(x)

poly/poly_test.v Outdated
println('Divide quotient: ${quotient}')
println('Divide remainder: ${remainder}')
assert quotient == [1.0, 1.0]
assert remainder == [] // boş küme bu iki polinomun tam bölündüğünü(kalansız bölündüğünü) ifade eder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

english please 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry, i'll fixit 😅

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (19)
ml/random_forest_test.v (4)

5-12: Suggestion: Add edge case tests and more assertions.

The test function verifies the creation of a RandomForest object but could include edge case tests (e.g., zero or negative values) and additional assertions to ensure robustness.

+  // Add edge case tests
+  rf_zero := RandomForest.new(0, 0, 0, 0)
+  assert rf_zero.n_trees == 0
+  assert rf_zero.max_depth == 0
+  assert rf_zero.min_samples_split == 0
+  assert rf_zero.feature_subset_size == 0
+  assert rf_zero.trees.len == 0

14-24: Suggestion: Add assertions to verify the integrity of the sampled data.

The test function verifies the bootstrap sample size but could include additional assertions to check the integrity of the sampled data.

+  // Add assertions to verify the integrity of the sampled data
+  for sample in bootstrap.samples {
+    assert dataset.samples.contains(sample)
+  }

34-54: Suggestion: Add assertions to verify prediction accuracy.

The test function verifies the training and prediction capabilities but could include additional assertions to check the accuracy of the predictions.

+  // Add assertions to verify prediction accuracy
+  mut correct_predictions := 0
+  for i in 0 .. 100 {
+    features := [f64(i * 10), f64(i * 20), f64(i * 30), f64(i * 40)]
+    prediction := rf.predict(features)
+    if prediction == (i % 2) {
+      correct_predictions += 1
+    }
+  }
+  assert correct_predictions > 80 // Example threshold for accuracy

56-62: Suggestion: Improve the success message.

The success message could be more informative by including the number of tests passed.

-  println('All Random Forest tests passed successfully!')
+  println('${__FUNCTION__.len} Random Forest tests passed successfully!')
ml/decision_tree_test.v (4)

5-11: Suggestion: Add edge case tests and more assertions.

The test function verifies the creation of a DecisionTree object but could include edge case tests (e.g., zero or negative values) and additional assertions to ensure robustness.

+  // Add edge case tests
+  dt_zero := DecisionTree.new(0, 0)
+  assert dt_zero.max_depth == 0
+  assert dt_zero.min_samples_split == 0

22-34: Suggestion: Add assertions to verify the integrity of the added samples.

The test function verifies the addition of samples but could include additional assertions to check the integrity of the added samples.

+  // Add assertions to verify the integrity of the added samples
+  for sample in dataset.samples {
+    assert sample.features.len == 3
+    assert sample.label >= 0 && sample.label < 4
+  }

49-62: Suggestion: Add assertions to verify prediction accuracy.

The test function verifies the training and prediction capabilities but could include additional assertions to check the accuracy of the predictions.

+  // Add assertions to verify prediction accuracy
+  mut correct_predictions := 0
+  for sample in dataset.samples {
+    prediction := dt.predict(sample.features)
+    if prediction == sample.label {
+      correct_predictions += 1
+    }
+  }
+  assert correct_predictions > 3 // Example threshold for accuracy

86-93: Suggestion: Improve the success message.

The success message could be more informative by including the number of tests passed.

-  println('All Decision Tree tests passed successfully!')
+  println('${__FUNCTION__.len} Decision Tree tests passed successfully!')
ml/svm_test.v (3)

76-117: Suggestion: Add assertions to verify prediction accuracy.

The test function verifies the SVM training and prediction but could include additional assertions to check the accuracy of the predictions.

+  // Add assertions to verify prediction accuracy
+  mut correct_predictions := 0
+  for point in data {
+    pred := predict(model, point.x)
+    if pred == point.y {
+      correct_predictions += 1
+    }
+  }
+  assert correct_predictions > 3 // Example threshold for accuracy

119-168: Suggestion: Add assertions to verify prediction accuracy.

The test function verifies the multiclass SVM training and prediction but could include additional assertions to check the accuracy of the predictions.

+  // Add assertions to verify prediction accuracy
+  mut correct_predictions := 0
+  for point in data {
+    predicted_class := predict_multiclass(multiclass_model, point.x)
+    if predicted_class == point.y {
+      correct_predictions += 1
+    }
+  }
+  assert correct_predictions > 3 // Example threshold for accuracy

170-180: Suggestion: Improve the success message.

The success message could be more informative by including the number of tests passed.

-  println('All SVM tests passed successfully!')
+  println('${__FUNCTION__.len} SVM tests passed successfully!')
poly/README.md (2)

46-50: Approved: Improved readability and grammar.

The changes enhance the clarity and grammatical correctness of the description.

Apply grammatical improvements.

Add a comma after "i.e." and "for example" for better readability.

- If one real root is found (i.e. if `a=0`) then it is returned as `[ x0 ]`.
+ If one real root is found (i.e., if `a=0`) then it is returned as `[ x0 ]`.

- For example `(x-1)^2=0` will have two roots, which happen to have exactly equal values.
+ For example, `(x-1)^2=0` will have two roots, which happen to have exactly equal values.
Tools
LanguageTool

[typographical] ~48-~48: Consider adding a comma.
Context: ...f one real root is found (i.e. if a=0) then it is returned as [ x0 ]. When two re...

(IF_THEN_COMMA)


[typographical] ~50-~50: After the expression ‘for example’ a comma is usually used.
Context: ...nt roots is not considered special. For example (x-1)^2=0 will have two roots, which ...

(COMMA_FOR_EXAMPLE)


70-75: Approved: Improved readability and style.

The changes enhance the clarity and readability of the description.

Apply style improvement.

Remove the unnecessary hyphen in "closely-spaced".

- finite precision may cause equal or closely-spaced real roots to move off the real axis
+ finite precision may cause equal or closely spaced real roots to move off the real axis
Tools
LanguageTool

[style] ~75-~75: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...se, finite precision may cause equal or closely-spaced real roots to move off the real axis in...

(HYPHENATED_LY_ADVERB_ADJECTIVE)

ml/random_forest.v (3)

24-32: Approved: Correct bootstrap sample function.

The bootstrap_sample function correctly creates a bootstrap sample from the given dataset.

Add error handling for rand.intn.

The use of rand.intn without error handling might lead to unexpected behavior. Consider adding error handling.

- sample_index := rand.intn(dataset.samples.len) or { 0 }
+ sample_index := rand.intn(dataset.samples.len) or { panic("Failed to generate random index") }

40-50: Approved: Correct training function.

The RandomForest.train function correctly trains the random forest on the given dataset.

Add logging for debugging.

Consider adding logging to help with debugging during the training process.

+ println("Training tree \${_ + 1} of \${rf.n_trees}")

52-65: Approved: Correct prediction function.

The RandomForest.predict function correctly predicts the class label for the given features using the trained random forest.

Use a more descriptive sentinel value.

Consider using a more descriptive sentinel value instead of -1 for no trees.

- return -1
+ return -9999 // No trees available
ml/decision_tree.v (3)

62-71: Approved: Correct sample addition function.

The Dataset.add_sample function correctly adds a sample to the dataset.

Improve return type for better readability.

Consider using a more descriptive return type, such as Result<bool, String>, for better readability and error handling.

- pub fn (mut dataset Dataset) add_sample(features []f64, label int) bool {
+ pub fn (mut dataset Dataset) add_sample(features []f64, label int) Result<bool, String> {

89-124: Approved: Correct best split function.

The find_best_split function correctly finds the best split for the dataset.

Add logging for debugging.

Consider adding logging to help with debugging during the split finding process.

+ println("Evaluating feature \${feature} with threshold \${threshold}")

126-179: Approved: Correct tree building function.

The build_tree function correctly builds a decision tree from the dataset.

Add logging for debugging.

Consider adding logging to help with debugging during the tree building process.

+ println("Building tree node with feature \${best_feature} and threshold \${best_threshold}")
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e578ca5 and 8801dcb.

Files selected for processing (9)
  • ml/decision_tree.v (1 hunks)
  • ml/decision_tree_test.v (1 hunks)
  • ml/random_forest.v (1 hunks)
  • ml/random_forest_test.v (1 hunks)
  • ml/svm.v (1 hunks)
  • ml/svm_test.v (1 hunks)
  • poly/README.md (4 hunks)
  • poly/poly.v (10 hunks)
  • poly/poly_test.v (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • poly/poly_test.v
Additional context used
LanguageTool
poly/README.md

[typographical] ~48-~48: Consider adding a comma.
Context: ...f one real root is found (i.e. if a=0) then it is returned as [ x0 ]. When two re...

(IF_THEN_COMMA)


[typographical] ~50-~50: After the expression ‘for example’ a comma is usually used.
Context: ...nt roots is not considered special. For example (x-1)^2=0 will have two roots, which ...

(COMMA_FOR_EXAMPLE)


[typographical] ~52-~52: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...depends on the sign of the discriminant b^2 - 4 a c. This will be subject to rounding...

(DASH_RULE)


[style] ~75-~75: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...se, finite precision may cause equal or closely-spaced real roots to move off the real axis in...

(HYPHENATED_LY_ADVERB_ADJECTIVE)

Markdownlint
poly/README.md

92-92: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Additional comments not posted (47)
ml/random_forest_test.v (1)

26-32: LGTM!

The test function correctly verifies the feature subset selection method.

ml/decision_tree_test.v (3)

13-20: LGTM!

The test function correctly verifies the creation of a dataset object.


36-47: LGTM!

The test function correctly verifies the entropy calculation method.


64-84: LGTM!

The test function correctly verifies the information gain calculation method.

ml/svm_test.v (7)

6-13: LGTM!

The test function correctly verifies the linear kernel function.


15-24: LGTM!

The test function correctly verifies the polynomial kernel function.


26-36: LGTM!

The test function correctly verifies the RBF kernel function.


38-45: LGTM!

The test function correctly verifies the quadratic kernel function.


47-56: LGTM!

The test function correctly verifies the custom kernel function.


58-65: LGTM!

The test function correctly verifies the dot product function.


67-74: LGTM!

The test function correctly verifies the vector subtraction function.

poly/README.md (6)

22-32: Approved: Improved clarity and consistency.

The restructuring of the eval_derivs function description enhances readability and consistency.


78-98: Approved: Clear and comprehensive description.

The new section provides a clear and comprehensive overview of the companion_matrix function.

Tools
Markdownlint

92-92: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


100-108: Approved: Clear and comprehensive description.

The new section provides a clear and comprehensive overview of the balance_companion_matrix function.


110-123: Approved: Clear and comprehensive description.

The new section provides a clear and comprehensive overview of the add function.


124-135: Approved: Clear and comprehensive description.

The new section provides a clear and comprehensive overview of the subtract function.


136-159: Approved: Clear and comprehensive descriptions.

The new sections provide clear and comprehensive overviews of the multiply and divide functions.

ml/random_forest.v (3)

14-22: Approved: Correct initialization function.

The RandomForest.new function correctly initializes a new RandomForest instance with the specified parameters.


34-38: Approved: Correct feature selection function.

The select_feature_subset function correctly selects a subset of features from the given number of features.


67-70: Approved: Correct training function with feature subset.

The DecisionTree.train_with_feature_subset function correctly trains the decision tree with a subset of features from the given dataset.

ml/decision_tree.v (5)

36-42: Approved: Correct initialization function.

The DecisionTree.new function correctly initializes a new DecisionTree instance with the specified parameters.


44-52: Approved: Correct maximum index function.

The index_of_max function correctly finds the index of the maximum value in an array.


54-60: Approved: Correct dataset creation function.

The create_dataset function correctly creates a new dataset with the specified number of features and classes.


73-87: Approved: Correct entropy calculation function.

The Dataset.calculate_entropy function correctly calculates the entropy of the dataset.


182-184: Approved: Correct training function.

The DecisionTree.train function correctly trains the decision tree on the given dataset.

ml/svm.v (10)

6-14: LGTM!

The SVMConfig struct is well-defined with appropriate default values.


16-22: LGTM!

The KernelType enum is comprehensive and includes a custom option for flexibility.


24-24: LGTM!

The KernelFunction type alias correctly defines the expected function signature for kernel functions.


26-30: LGTM!

The DataPoint struct is simple and correctly represents a data point for SVM training.


32-39: LGTM!

The SVMModel struct is well-defined and includes all necessary fields for an SVM model.


41-44: LGTM!

The MulticlassSVM struct correctly represents a multi-class SVM model.


46-72: LGTM!

The kernel functions are correctly implemented and follow standard definitions for SVM kernels. The custom kernel function is a unique addition.


74-88: LGTM!

The helper functions dot_product and vector_subtract are correctly implemented and essential for kernel computations.


90-177: LGTM!

The train_svm function comprehensively implements the SMO algorithm for training an SVM model.


178-254: LGTM!

The remaining functions are correctly implemented and essential for the SVM model's functionality. The train_multiclass_svm and predict_multiclass functions effectively extend the SVM model to support multi-class classification.

poly/poly.v (12)

Line range hint 9-24:
LGTM!

The eval function is correctly implemented and the documentation is clear and concise.


Line range hint 25-58:
LGTM!

The eval_derivs function is correctly implemented and the documentation is clear and concise.


Line range hint 59-89:
LGTM!

The solve_quadratic function is correctly implemented and the documentation is clear and concise.


Line range hint 90-129:
LGTM!

The solve_cubic function is correctly implemented and the documentation is clear and concise.


130-136: LGTM!

The swap_ function is correctly implemented and the documentation is clear and concise.


Line range hint 138-156:
LGTM!

The sorted_3_ function is correctly implemented and the documentation is clear and concise.


Line range hint 158-184:
LGTM!

The companion_matrix function is correctly implemented and the documentation is clear and concise.


Line range hint 185-249:
LGTM!

The balance_companion_matrix function is correctly implemented and the documentation is clear and concise.


250-259: LGTM!

The add function is correctly implemented and the documentation is clear and concise.


261-270: LGTM!

The subtract function is correctly implemented and the documentation is clear and concise.


272-283: LGTM!

The multiply function is correctly implemented and the documentation is clear and concise.


285-311: LGTM!

The divide function is correctly implemented and the documentation is clear and concise.

This reverts commit 4aba549.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8801dcb and 7ee9f4f.

Files selected for processing (1)
  • poly/README.md (4 hunks)
Additional context used
Markdownlint
poly/README.md

22-22: Expected: atx; Actual: setext
Heading style

(MD003, heading-style)


22-22: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


25-25: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


57-57: Expected: atx; Actual: setext
Heading style

(MD003, heading-style)


57-57: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


61-61: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


80-80: Expected: atx; Actual: setext
Heading style

(MD003, heading-style)


80-80: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


87-87: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


106-106: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

LanguageTool
poly/README.md

[uncategorized] ~52-~52: Possible missing comma found.
Context: ...d as [ x0, x1 ]. If no real roots are found then [] is returned. If one real root...

(AI_HYDRA_LEO_MISSING_COMMA)


[typographical] ~53-~53: Consider adding a comma.
Context: ...f one real root is found (i.e. if a=0) then it is returned as [ x0 ]. When two re...

(IF_THEN_COMMA)


[typographical] ~55-~55: After the expression ‘for example’ a comma is usually used.
Context: ...nt roots is not considered special. For example (x-1)^2=0 will have two roots, which ...

(COMMA_FOR_EXAMPLE)


[typographical] ~57-~57: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...depends on the sign of the discriminant b^2 - 4 a c. This will be subject to rounding...

(DASH_RULE)


[uncategorized] ~63-~63: Possible missing comma found.
Context: ...d as [ x0, x1 ]. If no real roots are found then [] is returned. If one real root...

(AI_HYDRA_LEO_MISSING_COMMA)


[typographical] ~65-~65: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...depends on the sign of the discriminant b^2 - 4 a c. This will be subject to rounding...

(DASH_RULE)


[style] ~86-~86: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...se, finite precision may cause equal or closely-spaced real roots to move off the real axis in...

(HYPHENATED_LY_ADVERB_ADJECTIVE)


[style] ~89-~89: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...se, finite precision may cause equal or closely-spaced real roots to move off the real axis in...

(HYPHENATED_LY_ADVERB_ADJECTIVE)

Additional comments not posted (2)
poly/README.md (2)

3-3: Fix heading style and punctuation.

The heading style should be consistent and the punctuation should be corrected.

-This chapter describes functions for evaluating and solving polynomials. There are routines
+This chapter describes functions for evaluating and solving polynomials. There are routines.

Likely invalid or redundant comment.


57-61: Fix heading style, punctuation, and use en dash for ranges.

The heading style should be consistent, punctuation should be corrected, and en dash should be used for ranges.

-The number of roots found depends on the sign of the discriminant `b^2 - 4 a c`. This will
-be subject to rounding and cancellation errors when computed in double precision, and will
-also be subject to errors if the coefficients of the polynomial are inexact. These errors may
-cause a discrete change in the number of roots. However, for polynomials with small integer
-coefficients the discriminant can always be computed exactly.
+The number of roots found depends on the sign of the discriminant `b^2 – 4 a c`. This will
+be subject to rounding and cancellation errors when computed in double precision, and will
+also be subject to errors if the coefficients of the polynomial are inexact. These errors may
+cause a discrete change in the number of roots. However, for polynomials with small integer
+coefficients, the discriminant can always be computed exactly.

Likely invalid or redundant comment.

Tools
LanguageTool

[typographical] ~57-~57: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...depends on the sign of the discriminant b^2 - 4 a c. This will be subject to rounding...

(DASH_RULE)

Markdownlint

57-57: Expected: atx; Actual: setext
Heading style

(MD003, heading-style)


57-57: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


61-61: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)


The companion matrix `C` is defined as:

```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify the language for fenced code blocks.

The language should be specified for fenced code blocks to improve readability.

-```
+```v
Tools
Markdownlint

106-106: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


## Balanced Companion Matrix

```v ignore
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify the language for fenced code blocks.

The language should be specified for fenced code blocks to improve readability.

-```
+```v


## Polynomial Operations

```v ignore
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify the language for fenced code blocks.

The language should be specified for fenced code blocks to improve readability.

-```
+```v


Returns the result as `[a_0 + b_0, a_1 + b_1, ..., max(a_k, b_k), ...]`.

```v ignore
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify the language for fenced code blocks.

The language should be specified for fenced code blocks to improve readability.

-```
+```v


Returns the result as `[a_0 - b_0, a_1 - b_1, ..., a_k - b_k, ...]`.

```v ignore
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify the language for fenced code blocks.

The language should be specified for fenced code blocks to improve readability.

-```
+```v


Returns the result as `[c_0, c_1, ..., c_{n+m}]` where `c_k = ∑_{i+j=k} a_i * b_j`.

```v ignore
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify the language for fenced code blocks.

The language should be specified for fenced code blocks to improve readability.

-```
+```v

fn divide(a []f64, b []f64) ([]f64, []f64)
```

Divides two polynomials:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify the language for fenced code blocks.

The language should be specified for fenced code blocks to improve readability.

-```
+```v

Comment on lines +80 to +87
<<<<<<< HEAD
with a leading coefficient of unity. The number of real roots (either one or three) is
returned, and their locations are returned as `[ x0, x1, x2 ]`. If one real root is found
then only `[ x0 ]` is returned. When three real roots are found they are returned as
`[ x0, x1, x2 ]` in ascending order. The case of coincident roots is not considered special.
For example, the equation `(x-1)^3=0` will have three roots with exactly equal values. As
in the quadratic case, finite precision may cause equal or closely-spaced real roots to move
off the real axis into the complex plane, leading to a discrete change in the number of real roots.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix heading style, punctuation, and hyphenated adverbs.

The heading style should be consistent, punctuation should be corrected, and hyphenated adverbs should be addressed.

-with a leading coefficient of unity. The number of real roots (either one or three) is
-returned, and their locations are returned as `[ x0, x1, x2 ]`. If one real root is found
-then only `[ x0 ]` is returned. When three real roots are found they are returned as
-`[ x0, x1, x2 ]` in ascending order. The case of coincident roots is not considered special.
-For example, the equation `(x-1)^3=0` will have three roots with exactly equal values. As
-in the quadratic case, finite precision may cause equal or closely-spaced real roots to move
-off the real axis into the complex plane, leading to a discrete change in the number of real roots.
+with a leading coefficient of unity. The number of real roots (either one or three) is
+returned, and their locations are returned as `[ x0, x1, x2 ]`. If one real root is found,
+only `[ x0 ]` is returned. When three real roots are found, they are returned as
+`[ x0, x1, x2 ]` in ascending order. The case of coincident roots is not considered special.
+For example, the equation `(x-1)^3=0` will have three roots with exactly equal values. As
+in the quadratic case, finite precision may cause equal or closely spaced real roots to move
+off the real axis into the complex plane, leading to a discrete change in the number of real roots.

Committable suggestion was skipped due to low confidence.

Tools
LanguageTool

[style] ~86-~86: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...se, finite precision may cause equal or closely-spaced real roots to move off the real axis in...

(HYPHENATED_LY_ADVERB_ADJECTIVE)

Markdownlint

80-80: Expected: atx; Actual: setext
Heading style

(MD003, heading-style)


80-80: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


87-87: Punctuation: '.'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)

@@ -20,16 +19,21 @@
fn eval(c []f64, x f64) f64
```

<<<<<<< HEAD
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix heading style and punctuation.

The heading style should be consistent and the punctuation should be corrected.

-This function evaluates a polynomial and its derivatives, storing the results in the array
+This function evaluates a polynomial and its derivatives, storing the results in the array.

Committable suggestion was skipped due to low confidence.

Tools
Markdownlint

22-22: Expected: atx; Actual: setext
Heading style

(MD003, heading-style)


22-22: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)

Comment on lines +50 to +55
<<<<<<< HEAD
The number of real roots (either zero, one or two) is returned, and their locations are
returned as `[ x0, x1 ]`. If no real roots are found then `[]` is returned. If one real root
is found (i.e. if `a=0`) then it is returned as `[ x0 ]`. When two real roots are found they
are returned as `[ x0, x1 ]` in ascending order. The case of coincident roots is not considered
special. For example `(x-1)^2=0` will have two roots, which happen to have exactly equal values.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix heading style, punctuation, and missing commas.

The heading style should be consistent, punctuation should be corrected, and missing commas should be added.

-The number of real roots (either zero, one or two) is returned, and their locations are
-returned as `[ x0, x1 ]`. If no real roots are found then `[]` is returned. If one real root
-is found (i.e. if `a=0`) then it is returned as `[ x0 ]`. When two real roots are found they
-are returned as `[ x0, x1 ]` in ascending order. The case of coincident roots is not considered
-special. For example `(x-1)^2=0` will have two roots, which happen to have exactly equal values.
+The number of real roots (either zero, one, or two) is returned, and their locations are
+returned as `[ x0, x1 ]`. If no real roots are found, then `[]` is returned. If one real root
+is found (i.e., if `a=0`), then it is returned as `[ x0 ]`. When two real roots are found, they
+are returned as `[ x0, x1 ]` in ascending order. The case of coincident roots is not considered
+special. For example, `(x-1)^2=0` will have two roots, which happen to have exactly equal values.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<<<<<<< HEAD
The number of real roots (either zero, one or two) is returned, and their locations are
returned as `[ x0, x1 ]`. If no real roots are found then `[]` is returned. If one real root
is found (i.e. if `a=0`) then it is returned as `[ x0 ]`. When two real roots are found they
are returned as `[ x0, x1 ]` in ascending order. The case of coincident roots is not considered
special. For example `(x-1)^2=0` will have two roots, which happen to have exactly equal values.
The number of real roots (either zero, one, or two) is returned, and their locations are
returned as `[ x0, x1 ]`. If no real roots are found, then `[]` is returned. If one real root
is found (i.e., if `a=0`), then it is returned as `[ x0 ]`. When two real roots are found, they
are returned as `[ x0, x1 ]` in ascending order. The case of coincident roots is not considered
special. For example, `(x-1)^2=0` will have two roots, which happen to have exactly equal values.
Tools
LanguageTool

[uncategorized] ~52-~52: Possible missing comma found.
Context: ...d as [ x0, x1 ]. If no real roots are found then [] is returned. If one real root...

(AI_HYDRA_LEO_MISSING_COMMA)


[typographical] ~53-~53: Consider adding a comma.
Context: ...f one real root is found (i.e. if a=0) then it is returned as [ x0 ]. When two re...

(IF_THEN_COMMA)


[typographical] ~55-~55: After the expression ‘for example’ a comma is usually used.
Context: ...nt roots is not considered special. For example (x-1)^2=0 will have two roots, which ...

(COMMA_FOR_EXAMPLE)

@suleyman-kaya suleyman-kaya deleted the add-func-poly branch August 4, 2024 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants